Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with failing NodeJS component after the container gets restarted #2265

Merged
merged 2 commits into from Oct 11, 2019

Conversation

kadel
Copy link
Member

@kadel kadel commented Oct 9, 2019

What is the purpose of this change? What does it change?

This fixes the issue where the source code for nodejs was lost after the container was restarted.
This was due to the fact that we removed steps that were coping source files from the /tmp/src directory to /opt/app-root/src.

This updates just the image used by odo, the actual fix is in odo-init-image - redhat-developer/odo-init-image#40

After this is tested. We will release a new odo-init-image, and update this PR to use the released version.

Was the change discussed in an issue?

fixes #2224

How to test changes?

You can download odo binaries build from this PR at https://gcsweb-ci.svc.ci.openshift.org/gcs/origin-ci-test/pr-logs/pull/openshift_odo/2265/pull-ci-openshift-odo-master-v4.1-unit/407/artifacts/unit/dist/bin/

odo component create nodejs
odo url create
odo push
odo config set --env FOO=vv
odo push
# verify that application is still running, previously it did not

or

cd frontend
odo component create nodejs frontend
odo url create
odo push

cd backend
odo component create java backend
odo push

cd frontend
odo link backend --port 8080

# verify that the frontend is running, previously it did not

TODO:

  • add proper tests

@kadel kadel changed the title fix dev-scripts Fix issue with failing NodeJS component after the container gets restarted Oct 9, 2019
@kadel kadel added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Oct 9, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Oct 9, 2019
@cdrage
Copy link
Member

cdrage commented Oct 9, 2019

I tested this as well with service listing:

~/nodejs-ex  master ✗                                                                                                          358d ◒  
▶ odo config set --env FOO=BAR
Environment variables were successfully updated.
Run `odo push --config` command to apply changes to the cluster.

~/nodejs-ex  master ✗                                                                                                          358d ◒  
▶ odo push --config
Validation
 ✓  Checking component [21ms]

Configuration changes
 ✓  Retrieving component data [12ms]
 ✓  Applying configuration [12s]

Applying URL changes
 ✓  URL nodejs-nodejs-ex-gymi-8080 already exists

~/nodejs-ex  master ✗                                                                                                          358d ◒  
▶ odo service create mongodb-persistent --plan default --wait -p DATABASE_SERVICE_NAME=mongodb -p MEMORY_LIMIT=512Mi -p MONGODB_DATABASE=sampledb -p VOLUME_CAPACITY=1Gi
 ✓  Creating service [88ms]
 ✓  Waiting for service to come up [37s]
 ✓  Service 'mongodb-persistent' is ready for use

~/nodejs-ex  master ✗                                                                                                          358d ◒  
▶ odo url list
Found the following URLs for component nodejs-nodejs-ex-gymi in application app:
NAME                           STATE      URL                                                                      PORT
nodejs-nodejs-ex-gymi-8080     Pushed     http://nodejs-nodejs-ex-gymi-8080-app-myproject.192.168.42.79.nip.io     8080

~/nodejs-ex  master ✗                                                                                                          358d ◒  
▶ odo link mongodb-persistent
 ✓  Service mongodb-persistent has been successfully linked from the component nodejs-nodejs-ex-gymi

Following environment variables were added to nodejs-nodejs-ex-gymi component:
- admin_password
- database_name
- password
- uri
- username

~/nodejs-ex  master ✗                                                                                                          358d ◒  
▶ ocget pods

~/nodejs-ex  master ✗                                                                                                         358d ◒  ⍉
▶ oc get pods
NAME                                READY     STATUS    RESTARTS   AGE
mongodb-1-2h6mj                     1/1       Running   0          5m
nodejs-nodejs-ex-gymi-app-3-xhfgl   1/1       Running   0          18s

~/nodejs-ex  master ✗                                                                                                          358d ◒  
▶ 

Works and the application comes up after restarting.

@jankleinert
Copy link
Contributor

I'm seeing an issue that I didn't see in a previous version.

git clone https://github.com/openshift-evangelists/Wild-West-Backend
cd Wild-West-Backend
mvn clean package
odo login
odo project create ododemo
odo create java backend --binary target/wildwest-1.0.jar

gives this error:

✗ Rel: can't make target/wildwest-1.0.jar relative to /Users/jkleiner/Downloads/Wild-West-Backend

I did not get that error in odo v1.0.0-beta6 (0cdcc90)

@girishramnani
Copy link
Contributor

girishramnani commented Oct 9, 2019

@jankleinert just to test this PR can you try /Users/jkleiner/Downloads/Wild-West-Backend/target/wildwest-1.0.jar as the --binary's input?

@jankleinert
Copy link
Contributor

@girishramnani with that workaround, it did work for me

@kadel
Copy link
Member Author

kadel commented Oct 10, 2019

/retest

failed to acquire a resource: Post http://boskos.ci/acquire?dest=leased&owner=ci-op-j758c1yk-1b2f4&request_id=4831054931297018086&state=free&type=aws-quota-slice: dial tcp 172.30.131.17:80: connect: no route to host

@kadel kadel added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. label Oct 10, 2019
@kadel kadel force-pushed the fix-dev-scripts branch 2 times, most recently from b1d5140 to 936aaec Compare October 10, 2019 07:57
@kadel
Copy link
Member Author

kadel commented Oct 10, 2019

I'm seeing an issue that I didn't see in a previous version.

git clone https://github.com/openshift-evangelists/Wild-West-Backend
cd Wild-West-Backend
mvn clean package
odo login
odo project create ododemo
odo create java backend --binary target/wildwest-1.0.jar

gives this error:

✗ Rel: can't make target/wildwest-1.0.jar relative to /Users/jkleiner/Downloads/Wild-West-Backend

I did not get that error in odo v1.0.0-beta6 (0cdcc90)

I thought that I saw this error somewhere. Now I remembered that I've opened PR to fix this last week. - #2211 It is just waiting to be merged

@kadel
Copy link
Member Author

kadel commented Oct 10, 2019

There is one big problem with this and that is that re-introduces #2171

@kadel
Copy link
Member Author

kadel commented Oct 10, 2019

There is one big problem with this and that is that re-introduces #2171

It looks like that cp -p flag is causing this (but only with files that come from Windows).
Fixed by simply removing the flag redhat-developer/odo-init-image@4736f9b
The flag is trying to preserve some file attributes like file modification times, access times, file flags etc. My suspicion is that some of the attributes might not be compatible with volume mount on OpenShift.

Copy link
Member

@dharmit dharmit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

Works for me as mentioned in the steps to test. Code LGTM.

/hold so that this doesn't go in before image name is changed after redhat-developer/odo-init-image#40 goes in.

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dharmit

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Oct 10, 2019
Copy link
Member

@cdrage cdrage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from me as well @dharmit

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Oct 10, 2019
@kadel
Copy link
Member Author

kadel commented Oct 11, 2019

/retest

[odo]  ✗  waited 4m0s but couldn't find running pod matching selector: 'deploymentconfig=javaee-war-test-app'

@kadel
Copy link
Member Author

kadel commented Oct 11, 2019

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. label Oct 11, 2019
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 8f7d544 into redhat-developer:master Oct 11, 2019
@rm3l rm3l added the estimated-size/S (5-10) Rough sizing for Epics. Less then one sprint of work for one person label Jun 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. estimated-size/S (5-10) Rough sizing for Epics. Less then one sprint of work for one person lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

odo link Fails to Link Java Component to Node.js Component
9 participants